refactor: use normalizeLicense util consistently#2816
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR applies the shared ChangesLicense normalisation application
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hello! Thank you for opening your first PR to npmx, @charpeni! 🚀 Here’s what will happen next:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/registry/license-change/[...pkg].get.ts (1)
41-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
currentVersionIndexbefore array access.The
currentVersionIndexcan be-1in two scenarios: (1) whenversion === 'latest'and theversionsarray is empty, or (2) whenfindIndexfails to locate the requested version. Accessingversions[currentVersionIndex]on line 49 with a negative index violates type-safety. As per coding guidelines, always check when accessing an array value by index.🛡️ Proposed fix to validate currentVersionIndex
const currentVersionIndex = version === 'latest' ? versions.length - 1 : versions.findIndex(v => v.version === version) +if (currentVersionIndex < 0) { + throw createError({ + statusCode: 404, + statusMessage: 'Version not found', + }) +} + const previousVersionIndex = currentVersionIndex - 1 if (previousVersionIndex < 0) { return { change } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/registry/license-change/`[...pkg].get.ts around lines 41 - 49, currentVersionIndex can be -1 (empty versions or findIndex miss), so before accessing versions[currentVersionIndex] in the assignment to currentLicense, validate currentVersionIndex is within [0, versions.length-1]; if it's -1 or out of range, set currentLicense to 'UNKNOWN' (or the existing fallback) and proceed—update the logic around the currentVersionIndex calculation and the currentLicense assignment to guard against negative indices and avoid unsafe array access.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server/api/registry/license-change/`[...pkg].get.ts:
- Around line 41-49: currentVersionIndex can be -1 (empty versions or findIndex
miss), so before accessing versions[currentVersionIndex] in the assignment to
currentLicense, validate currentVersionIndex is within [0, versions.length-1];
if it's -1 or out of range, set currentLicense to 'UNKNOWN' (or the existing
fallback) and proceed—update the logic around the currentVersionIndex
calculation and the currentLicense assignment to guard against negative indices
and avoid unsafe array access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 250d627f-c1e0-495f-b578-54f044d06a98
📒 Files selected for processing (2)
server/api/registry/license-change/[...pkg].get.tstest/unit/server/api/registry/license-change/pkg.get.spec.ts
|
We got a few PRs for this fix, so we merged all the changes into #2792 and closed the other ones - in this case though we can turn this PR into a refactor of the use of the normalise util 👀 |
6eea39b to
3036392
Compare
|
Thanks for your first contribution, @charpeni! ✨ We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role. |
🔗 Linked issue
n/a
🧭 Context
n/a
📚 Description
This refactor uses the
normalizeLicensefunction more consistently.